test(pi): validate package integrity in CI (PF-3852) by Zechereh · Pull Request #43 · tinyfish-io/tinyfish-web-agent-integrations

Zechereh · 2026-09-11T22:54:51Z

Stacked on #42. Answers "is there a way to test that this package works?"

The package ships no code, so nothing here fails loudly. That is the whole problem:

a malformed skill description → pi silently does not load that skill
a dropped files entry → the tarball publishes and installs, just with skills missing
a tool the skills teach but that is not in directTools → the model calls something never registered

None of it surfaces until a user hits it. These checks make each one fail at PR time instead.

`pi/test/validate.mjs`

Dependency-free, runs locally as node test/validate.mjs, excluded from the tarball.

Check	Catches
`manifest`	`pi.mcp`/`pi.skills` pointing at missing paths; a dropped `pi-package` keyword, which is the entire gallery discovery mechanism
`mcp`	`type: "http"` creeping back (the adapter has no such field); the `${VAR:-}` interpolation form, which ships as a literal header value; empty or duplicated `directTools`
`skills`	the frontmatter name/description rules pi enforces, name↔directory mismatch, duplicate names, and skill-relative `.md` references that do not resolve
`skills-vs-directTools`	a tool taught by the skills but not registered
`files`	a missing `files` entry, or `test/` leaking into the tarball

Every check is negative-tested — each was made to fire by introducing exactly the regression it guards, then reverted.

It found two real bugs, both fixed in the branches below

batch_status / batch_cancel were taught by the automation skill but missing from directTools — a model following that skill would have called tools that never registered. Fixed in feat(pi): package scaffold and MCP registration (PF-3852) #40 (now 10 tools).
tinyfish-authenticated cited references/anti-bot.md and references/goals.md, which live under tinyfish-automation and so did not resolve from where that skill sits. Fixed in feat(pi): TinyFish skills and README (PF-3852) #41 to use resolvable ../tinyfish-automation/... paths. grok/ still has this one — worth a separate look, not touched here.

That is two bugs in a package I had already verified by hand in a live pi session, which is the argument for the checks existing.

`pi-ci.yml`

Two jobs.

validate — runs the validator plus tarball verification on PRs. pi-publish.yml already verifies the tarball, but only on push to main, so nothing was validating pi/ on the PR that breaks it.

install — installs pi and pi-mcp-adapter the way a user does, registers the package, and asserts the adapter resolves server tiny-fish_pi__tinyfish with directTools intact. That is the contract the skills are written against, and it lives in someone else's package: if the adapter changes how it derives server names, that should break our build rather than our users. No model, no credentials, no network calls to us — deterministic and free. Verified locally by running the job's exact commands.

Deliberately not covered

Validating directTools names against the live MCP server's tools/list would catch product-side drift — a server-side rename silently stops a tool registering. It needs a TinyFish API key, and this repo has no such secret (ux-labs uses MINO_PROD_API_KEY). Worth a scheduled job later; flagging rather than half-building it.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CBf5rnVYQYcxE8bLjfQuUP

coderabbitai · 2026-09-11T22:54:59Z

Warning

Review limit reached

Run on-demand review

_{On-demand reviews are free for the next 6 days. After that, they cost $0.25 per reviewed file.}

Or wait 52 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used all 2 included reviews currently available. Your 55 included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Essentials

Run ID: 85b0cc03-6b14-4cfa-bfc6-0f2fc435e7f3

📥 Commits

Reviewing files that changed from the base of the PR and between e5666f0 and 8535936.

📒 Files selected for processing (2)

.github/workflows/pi-ci.yml
pi/test/validate.mjs

_{Comment @coderabbitai help to get the list of available commands.}

Zechereh · 2026-09-11T22:56:59Z

Note on why Pi CI shows no run here yet.

Every CI workflow in this repo filters on pull_request: branches: [main], which matches the base branch — hermes-ci.yml, langchain-ci.yml, google-adk-ci.yml and plugin-manifests-ci.yml all do. This PR's base is zach/pf-3852-pi-publish, so pi-ci.yml will first execute when the stack lands and this retargets main. Same reason #41 and #42 show only TruffleHog, while #40 (base main) picked up pi-versions-match.

Not a problem with the workflow, but it does mean GitHub hasn't executed it yet. I ran both jobs' exact commands locally instead:

$ node test/validate.mjs
  ok  manifest
  ok  mcp
  ok  skills (5 valid)
  ok  skills-vs-directTools (10 taught, all registered)
  ok  files
All checks passed (5).

$ export PI_CODING_AGENT_DIR="$(mktemp -d)"
$ pi install ./pi && pi install npm:pi-mcp-adapter
$ node -e "...loadPackageMcpConfigs..."
PASS: adapter resolved tiny-fish_pi__tinyfish with 10 direct tools

Worth a look at the first green Pi CI run once #40–#42 merge, rather than assuming from this.

londondavila · 2026-09-14T16:21:34Z

+
+on:
+  pull_request:
+    branches: [main]


this PR targets zach/pf-3852-pi-publish, so branches: [main] prevented both Pi jobs from running on the change that adds them. should PR validation run on stacked bases too, or is retargeting to main before merge enforced?

Fixed — and you are right that it bit the very PR that adds the jobs. Dropped the branches: filter from pull_request entirely, with a comment saying why: the filter matches the base, so stacked PRs get no validation until they retarget main. Kept branches: [main] on push. The other CI here keeps the filter; this one deliberately does not, since validation should run wherever pi/ changes.

londondavila · 2026-09-14T16:21:34Z

+              console.error('Expected server tiny-fish_pi__tinyfish, adapter resolved: ' + JSON.stringify(names));
+              process.exit(1);
+            }
+            const direct = cfg.mcpServers['tiny-fish_pi__tinyfish'].directTools;


this only proves at least one direct tool survived; an adapter returning ["search"] still passes the “intact” contract. should it compare the resolved set with pi/mcp.json?

Fixed. It compared truthiness, so ["search"] would have passed the "intact" contract exactly as you describe.

Now compares the resolved set against pi/mcp.json element-wise (sorted, JSON-compared) and prints both sides on mismatch. Verified both directions locally: real install passes with all 12 intact, and a simulated truncation to ["search"] fails.

londondavila · 2026-09-14T16:21:34Z

+if (dirs) {
+  const name = 'skills-vs-directTools';
+  const registered = new Set(mcp.mcpServers?.tinyfish?.directTools ?? []);
+  const KNOWN = [


the static vocabulary skips list_browser_sessions, so head reports “10 taught, all registered” while tinyfish-browser teaches an unregistered eleventh tool. should KNOWN and directTools include it (plus a regression case)?

Already fixed in the head you reviewed against — list_browser_sessions is in both KNOWN and directTools.

More usefully, the same gap suggested the structural fix: KNOWN could silently fall behind mcp.json again. Added a guard that fails when directTools contains any name KNOWN does not list. It immediately caught list_runs, which my fix for the async/retry thread had just introduced. directTools is now 12, verified against the live server: every name we list is a real tool, and five server tools are proxy-only by choice.

londondavila · 2026-09-14T16:21:34Z

+const NAME_RE = /^[a-z0-9]+(-[a-z0-9]+)*$/;
+const MAX_DESC = 1024;
+
+function parseFrontmatter(text) {


this parser treats description: | as literal | and folded 2,001-character text as >, while Pi’s YAML parser sees empty/over-limit values. should it parse YAML or reject block scalars?

Fixed, and thank you — this was the one I would not have found myself.

Rather than take a YAML dependency, the parser now rejects what it cannot measure: block scalars (|, >), anchors/aliases, and any continuation line fail with an explicit message. So the mis-measurement you describe becomes a loud failure instead of a silent pass. The comment at the top says exactly that, so nobody later "improves" it into a lenient parser.

Negative-tested: description: | + folded text now fails with "description" uses a block scalar (|); keep it a single-line plain scalar so its length can be checked.

londondavila · 2026-09-14T16:21:34Z

+              console.error('Missing from tarball:\n  ' + missing.join('\n  '));
+              process.exit(1);
+            }
+            if (paths.has('test/validate.mjs')) {


this rejects one filename, not test/: adding test/fixture.txt to files passes both new checks and ships it. should the tarball assertion reject any path starting with test/?

Fixed in both places. The tarball assertion now rejects any path equal to test or starting with test/, and prints what leaked; validate.mjs applies the same rule to every files entry rather than the single test string.

Negative-tested with test/fixture.txt in files — fails with "test/fixture.txt" would ship the test directory in the tarball.

londondavila · 2026-09-14T16:21:34Z

+    }
+
+    // Skill-relative paths, the bug class that shipped a dangling rules/security.md pointer.
+    for (const m of text.matchAll(/`([^`\s]+\.md)`/g)) {


this only recognizes backticked .md paths, so Pi’s documented [guide](references/file.md) form can dangle without failing. should the check validate Markdown link destinations too?

Fixed. Now collects both spellings — backticked paths and Markdown link destinations ](path.md) — and skips absolute, anchor, and http(s) targets.

Negative-tested with [the guide](references/does-not-exist.md), which now fails. Worth noting the original check did earn its place: it caught two dangling backticked refs in tinyfish-authenticated before this PR, so widening it closes the remaining half.

londondavila · 2026-09-14T16:21:38Z

+const MAX_DESC = 1024;
+
+function parseFrontmatter(text) {
+  const m = text.match(/^---\r?\n([\s\S]*?)\r?\n---/);


looks fragile

Agreed — it was. Rewritten per your line-23 comment: it now refuses block scalars, anchors/aliases and continuation lines outright rather than silently mis-parsing them, so any frontmatter shape it cannot measure correctly fails loudly. Same thread has the negative test.

Ports the five skills from `grok/` — router plus research, automation, authenticated and browser — with their `references/` subdirs, which pi supports natively. Prefixed names are kept deliberately: pi skills land in the shared `~/.agents/skills` namespace alongside every other package's, where `search` would be ambiguous and would also collide with the CLI-installed `use-tinyfish`. Most of the diff is a verbatim port. The adaptations are: | Change | Why | |---|---| | New "Finding the tools" section in the router | Same tool has three names depending on install path. The suffix is the tool, the prefix names the install. | | New CLI-fallback section with a mapping table | Pi ships no MCP client, so most users have no TinyFish tools at all. The CLI grammar is two-level (`tinyfish search query "<q>"`) and does not mirror the tool names, so without the table a model invents `tinyfish run_web_automation`. | | Rewrote both Auth sections | grok's said the server is "configured by this plugin, authenticated by OAuth on first connection" — the exact opposite of the truth on this route, which is key-only with no OAuth. | | `rules/security.md` -> `../../rules/security.md` | Pi resolves skill references relative to the skill directory, so the bare path dangled. | | "plugin" -> "package" throughout | This is an npm package, not a plugin; pi users would not recognise the term. | The auth failure mode is quieter than expected and the copy reflects it. With `TINYFISH_API_KEY` unset the server never finishes connecting, so no metadata cache is built and *no tools register at all* — no 401, no error text, nothing at startup. That is indistinguishable at a glance from having no adapter installed, so the router carries a two-branch diagnostic: no `mcp` tool at all means no adapter; `mcp` present but `mcp({ search: "tinyfish" })` empty means the key. Verified in an isolated `PI_CODING_AGENT_DIR` against a live pi session: all five skills load, all eight MCP tools register top-level, a real search call returns through the package's own registration, and with the adapter removed the model reaches `tinyfish search query "..."` unaided — then recovers to `npx -y @tiny-fish/cli@latest` when the binary is absent too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CBf5rnVYQYcxE8bLjfQuUP

Follows ux-labs CD_cli.yml rather than the PyPI workflows in this repo, because it is the house pattern for npm here. **Auth is npm Trusted Publisher (OIDC), so there is no NPM_TOKEN secret.** The job requests `id-token: write` and npm verifies the workflow identity directly. This repo has no npm secret and did not need to grow one. Bootstrap is manual and one-time, because a trusted publisher cannot be configured against a package that does not exist yet: 1. `cd pi && npm publish --access public` 2. npmjs.com -> @tiny-fish/pi -> Settings -> Trusted Publisher -> GitHub Actions, repo tinyfish-io/tinyfish-web-agent-integrations, workflow pi-publish.yml 3. every release after that is this workflow, triggered by a version bump Documented in the workflow header so the next person does not have to reconstruct it. Publishing is gated on the version in pi/package.json not already existing on the registry, so content-only edits do not require a version bump and a re-run is a no-op rather than an error. An unexpected registry response fails the job instead of reading as "already published". Borrowed from CD_cli.yml, and the most valuable part here: verifying tarball contents before publishing. This package ships no code, so a dropped `files` entry is the entire failure mode — the tarball still publishes and still installs, just with skills silently missing. The job asserts every SKILL.md, mcp.json, the README and rules/security.md are present, and that the references/ docs the skills link to came along. Negative-tested by removing `rules` from `files`, which fails the job naming the missing path. Node 24 for npm >= 11.5.1, which trusted publishing requires. Action refs use tags to match every other hand-written workflow here; the one pinned SHA in the repo is in Terraform-managed secrets-scanner.yml. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CBf5rnVYQYcxE8bLjfQuUP

The package ships no code, so nothing here fails loudly. A malformed skill description means pi silently does not load that skill; a dropped `files` entry means the tarball publishes and installs with skills missing; a tool the skills teach but that is not in `directTools` means the model calls something that was never registered. All three are invisible until a user hits them. `pi/test/validate.mjs` — dependency-free, runs locally as `node test/validate.mjs`: | Check | Catches | |---|---| | manifest | `pi.mcp`/`pi.skills` pointing at missing paths; a dropped `pi-package` keyword, which is the entire gallery discovery mechanism | | mcp | `type: "http"` creeping back (the adapter has no such field); the `${VAR:-}` interpolation form, which ships as a literal header; empty or duplicated `directTools` | | skills | frontmatter name/description rules pi enforces, name/directory mismatch, duplicate names, and skill-relative `.md` references that do not resolve | | skills-vs-directTools | a tool taught by the skills but not registered | | files | a missing `files` entry, or `test/` leaking into the tarball | Written after the check found two real bugs in the branch below it, both now fixed there: `batch_status`/`batch_cancel` were taught by the automation skill but absent from `directTools`, and `tinyfish-authenticated` cited `references/anti-bot.md` and `references/goals.md`, which live under `tinyfish-automation` and so did not resolve from where that skill sits. The second is inherited from `grok/`, which still has it. Every check is negative-tested: each one was made to fire by introducing exactly the regression it guards. `pi-ci.yml` runs the validator plus tarball verification on PRs — pi-publish.yml already checks the tarball, but only on push to main, so nothing was validating `pi/` on the PR that breaks it. The second job installs pi and `pi-mcp-adapter` the way a user does, registers the package, and asserts the adapter resolves server `tiny-fish_pi__tinyfish` with `directTools` intact. That is the contract the skills are written against and it lives in someone else's package, so a silent upstream change to the name derivation should break our build rather than our users. No model, no credentials, no network calls to us — deterministic and free. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CBf5rnVYQYcxE8bLjfQuUP

Zechereh requested review from KateZhang98 and londondavila September 11, 2026 22:59

londondavila reviewed Sep 14, 2026

View reviewed changes

londondavila approved these changes Sep 14, 2026

View reviewed changes

Zechereh force-pushed the zach/pf-3852-pi-publish branch from a45adb1 to af63024 Compare September 14, 2026 17:33

Zechereh force-pushed the zach/pf-3852-pi-tests branch from 966017e to 57034fe Compare September 14, 2026 17:33

Zechereh force-pushed the zach/pf-3852-pi-publish branch from af63024 to 7dd382a Compare September 14, 2026 17:35

Zechereh force-pushed the zach/pf-3852-pi-tests branch from 57034fe to df3b8fa Compare September 14, 2026 17:35

Zechereh force-pushed the zach/pf-3852-pi-publish branch from 7dd382a to fc00d50 Compare September 14, 2026 17:57

Zechereh force-pushed the zach/pf-3852-pi-tests branch from df3b8fa to 96eeeb2 Compare September 14, 2026 17:57

Zechereh force-pushed the zach/pf-3852-pi-publish branch from fc00d50 to 38347c1 Compare September 14, 2026 18:00

Zechereh force-pushed the zach/pf-3852-pi-tests branch from 96eeeb2 to 90f0fd3 Compare September 14, 2026 18:00

Zechereh force-pushed the zach/pf-3852-pi-publish branch from 38347c1 to 023b6ca Compare September 14, 2026 18:54

Zechereh force-pushed the zach/pf-3852-pi-tests branch from 90f0fd3 to 5404565 Compare September 14, 2026 18:54

Zechereh force-pushed the zach/pf-3852-pi-publish branch from 023b6ca to 1949f09 Compare September 14, 2026 19:03

Zechereh force-pushed the zach/pf-3852-pi-tests branch from 5404565 to 8d604b9 Compare September 14, 2026 19:03

Zechereh mentioned this pull request Sep 14, 2026

feat(pi): TinyFish skills and README (PF-3852) #41

Merged

Zechereh force-pushed the zach/pf-3852-pi-publish branch from 1949f09 to f581015 Compare September 14, 2026 19:13

Zechereh force-pushed the zach/pf-3852-pi-tests branch from 8d604b9 to bc22209 Compare September 14, 2026 19:14

Zechereh force-pushed the zach/pf-3852-pi-publish branch from f581015 to 15945df Compare September 14, 2026 19:28

Zechereh force-pushed the zach/pf-3852-pi-tests branch from bc22209 to ccc08a2 Compare September 14, 2026 19:28

Zechereh force-pushed the zach/pf-3852-pi-publish branch from 15945df to 4e7f2b2 Compare September 14, 2026 19:39

Zechereh force-pushed the zach/pf-3852-pi-tests branch from ccc08a2 to 7d95934 Compare September 14, 2026 19:39

Zechereh force-pushed the zach/pf-3852-pi-publish branch from 4e7f2b2 to 88789a7 Compare September 14, 2026 19:54

Zechereh force-pushed the zach/pf-3852-pi-tests branch from 7d95934 to 782c353 Compare September 14, 2026 19:54

Zechereh and others added 3 commits September 14, 2026 14:00

Zechereh force-pushed the zach/pf-3852-pi-publish branch from 88789a7 to fb470eb Compare September 14, 2026 21:01

Zechereh force-pushed the zach/pf-3852-pi-tests branch from 782c353 to 9dd6077 Compare September 14, 2026 21:01

Base automatically changed from zach/pf-3852-pi-publish to main September 14, 2026 21:07

Merge branch 'main' into zach/pf-3852-pi-tests

8535936

Zechereh merged commit 568f2b6 into main Sep 14, 2026
7 checks passed

Zechereh deleted the zach/pf-3852-pi-tests branch September 14, 2026 21:09

Conversation

Zechereh commented Sep 11, 2026

pi/test/validate.mjs

It found two real bugs, both fixed in the branches below

pi-ci.yml

Deliberately not covered

Uh oh!

coderabbitai Bot commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

Zechereh commented Sep 11, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

`pi/test/validate.mjs`

`pi-ci.yml`

coderabbitai Bot commented Sep 11, 2026 •

edited

Loading